Skip to content

[FLINK-40559][clients] Honor --claimMode in application mode - #29109

Open
minxhe wants to merge 4 commits into
apache:masterfrom
minxhe:flink-40559-claim-mode
Open

minxhe wants to merge 4 commits into
apache:masterfrom
minxhe:flink-40559-claim-mode

Conversation

@minxhe

@minxhe minxhe commented Sep 4, 2026 •

Copy link
Copy Markdown

What is the purpose of the change

Flink silently discards an explicitly-passed --claimMode (or its deprecated alias
--restoreMode), so the recovery claim mode falls back to NO_CLAIM with no warning.
This fixes FLINK-40559, including
the shared reconstruction issue reported in
FLINK-40562.
There are three causes:

  1. StandaloneApplicationClusterConfigurationParserFactory.getOptions() never registers
    SAVEPOINT_CLAIM_MODE / SAVEPOINT_RESTORE_MODE. The parser runs with
    stopAtNonOption = true, so an unrecognized --claimMode does not fail — it ends
    option parsing and the remainder is swallowed into getArgs().
  2. CliFrontendParser.createSavepointRestoreSettings() reads the claim mode only inside
    the branch guarded by --fromSavepoint. Passing --claimMode without a savepoint
    path returns SavepointRestoreSettings.none() and drops it. Application-mode
    JobManagers hit this on HA recovery, where the checkpoint comes from HA storage
    rather than from a command line path.
  3. SavepointRestoreSettings.fromConfiguration() returns none() when no savepoint
    path is present, dropping an explicitly configured mode during reconstruction in
    both application and session submission paths.

This is not about configuration being overwritten. Since
FLINK-39673 /
PR #28295, an unset claim
mode is no longer written to the Configuration, so a value from flink-conf.yaml
survives. The remaining defects concern CLI parsing and reconstructing pathless settings
from that configuration.

Brief change log

  • StandaloneApplicationClusterConfigurationParserFactory — register
    SAVEPOINT_CLAIM_MODE and SAVEPOINT_RESTORE_MODE.
  • CliFrontendParser — branch on option presence rather than on the presence of a
    savepoint path; extract parseRecoveryClaimMode(). The parsed mode is null exactly
    when neither option was passed, so an explicit --claimMode NO_CLAIM stays
    distinguishable from no override at all.
  • SavepointRestoreSettings — add forRecoveryClaimMode(...) for a claim mode with no
    restore path, and extend toString() to render that state (it previously printed
    none()). Preserve pathless modes in fromConfiguration() using getOptional,
    including the deprecated execution.savepoint-restore-mode alias and canonical-key
    precedence. Absent path and absent mode still return none(), including allow-only
    configuration.

allowNonRestoredState stays null unless the flag was passed, so the new branch does
not start writing execution.state-recovery.ignore-unclaimed-state where nothing was
written before (the "explicitly set" semantics from FLINK-39673).

Option registration, parser restructuring, and shared reconstruction are separate commits.

Application and session scope

ProgramOptions.applyToConfiguration() writes the parsed mode, and
ExecutionConfigAccessor.getSavepointRestoreSettings() reconstructs it through
SavepointRestoreSettings.fromConfiguration().

Application mode also reconstructs these settings: after
StandaloneApplicationClusterEntryPoint.loadConfigurationFromClusterConfig() writes
them to the configuration, StreamGraphGenerator reads them back, and
PipelineExecutorUtils.getStreamGraph() reads them through ExecutionConfigAccessor
and sets them on the graph again. The shared fix preserves pathless modes through
both paths; the earlier session-only limitation was incorrect.

This does not retroactively override persisted HA graphs. EmbeddedExecutor can
recover an existing job ID without submitting the newly generated graph.

Verifying this change

This change added tests and can be verified as follows:

  • StandaloneApplicationClusterConfigurationParserFactoryTest — --claimMode and
    --restoreMode are picked up by the application-mode parser, i.e. the options are
    registered.
  • CliFrontendRunTest — pathless CLI settings now pass through ProgramOptions and
    ExecutionConfigAccessor. Parameterized application-graph coverage asserts settings
    before and after PipelineExecutorUtils.getStreamGraph() for all three modes.
  • SavepointRestoreSettingsTest — fresh-configuration round trips preserve explicit
    modes and nullable/false/true allow settings; absent options and allow-only behavior
    remain unchanged. Deprecated-alias and canonical-key precedence are covered.
  • StreamGraphGeneratorTest — parameterized pathless-mode propagation through the
    existing graph-generator fixture.

The original parser defects were first reproduced as failing tests on unmodified
master (expected: CLAIM but was: NO_CLAIM) and confirmed independent. For the
shared reconstruction follow-up, the settings suite failed 15 cases and the graph
propagation test failed all 3 mode cases before the production fix; both passed afterward.
Dependent client tests were skipped in the graph red run and passed in the final run.

Latest local validation (JDK 17):

./mvnw -pl flink-runtime,flink-streaming-java,flink-clients,flink-container \
  -Dtest=SavepointRestoreSettingsTest,StreamGraphGeneratorTest,CliFrontendRunTest,StandaloneApplicationClusterConfigurationParserFactoryTest \
  -Dsurefire.failIfNoSpecifiedTests=false test spotless:check checkstyle:check

109 tests passed (31 settings, 32 graph-generator, 33 CLI, 13 container); no failures,
errors or skips. Spotless and Checkstyle passed. No live-cluster failover scenario or
full clean verify was run for this follow-up.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API, i.e., is any changed class annotated with @Public(Evolving): no
  • The serializers: no (no fields added; serialVersionUID unchanged)
  • The runtime per-record code paths (performance sensitive): no (startup and
    graph construction/submission only)
  • Anything that affects deployment or recovery: JobManager (and its components),
    Checkpointing, Kubernetes/Yarn, ZooKeeper: yes — changes how an explicit claim
    mode reaches the JobManager configuration and newly generated/submitted graphs.
    With neither a path nor an explicit mode, behavior remains unchanged. Explicit
    pathless modes are now honored instead of silently discarded.
  • The S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature? no — it fixes an existing CLI
    option that was silently ignored.
  • If yes, how is the feature documented? not applicable

Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: GitHub Copilot CLI 1.0.80; follow-up: GitHub Copilot CLI 1.0.84-5

…ation mode

StandaloneApplicationClusterConfigurationParserFactory.getOptions() did not
register SAVEPOINT_CLAIM_MODE or SAVEPOINT_RESTORE_MODE. Because the parser runs
with stopAtNonOption=true, an unrecognized --claimMode did not fail: it ended
option parsing and the remainder was swallowed into getArgs(). The claim mode was
therefore unreachable in application mode and fell back to NO_CLAIM.

Register both options so that createSavepointRestoreSettings can read them.
Behavior is unchanged for jobs that pass neither option.

Generated-by: GitHub Copilot CLI 1.0.80
createSavepointRestoreSettings read the claim mode only inside the branch guarded
by --fromSavepoint, so passing --claimMode without a savepoint path returned
SavepointRestoreSettings.none() and discarded it. Application-mode JobManagers hit
this on HA recovery, where the checkpoint comes from HA storage rather than from a
command line path.

Branch on option presence instead, and add
SavepointRestoreSettings.forRecoveryClaimMode for a claim mode with no restore
path. The parsed mode is null exactly when neither option was passed, so an
explicit --claimMode NO_CLAIM stays distinguishable from no override at all.

allowNonRestoredState stays null unless the flag was passed, so the new branch does
not start writing execution.state-recovery.ignore-unclaimed-state where nothing was
written before (FLINK-39673).

SavepointRestoreSettings.fromConfiguration has the same savepoint-path gate, so the
`flink run` path still drops a pathless claim mode. That asymmetry is pre-existing
and left untouched here; application mode writes directly to the JobManager
configuration and does not go through it.

Generated-by: GitHub Copilot CLI 1.0.80
@flinkbot

flinkbot commented Sep 4, 2026 •

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@spuru9

spuru9 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Can you remove the first section of the PR description.

Generated-by: GitHub Copilot CLI 1.0.83-5

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@minxhe

minxhe commented Sep 5, 2026

Copy link
Copy Markdown
Author

Can you remove the first section of the PR description.

thanks for the catch, have removed!

@minxhe
minxhe requested a review from spuru9 September 5, 2026 05:27
@github-actions github-actions Bot added the community-reviewed PR has been reviewed by the community. label Sep 5, 2026
@minxhe

minxhe commented Sep 10, 2026 •

Copy link
Copy Markdown
Author

@spuru9 Thanks for the review! I’ve combined the tests into a parameterized test and removed the introductory section from the PR description. Could you take another look when you have time? Please let me know if there are any remaining concerns with the scope or approach.

…reconstruction

Generated-by: GitHub Copilot CLI 1.0.84-5

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@minxhe
minxhe requested a review from Jackeyzhe September 15, 2026 23:49

@Jackeyzhe Jackeyzhe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the pathless recovery-mode round trip. I re-reviewed f214294 and confirmed that the explicitly configured claim mode now survives CLI parsing, configuration reconstruction, and StreamGraph creation.
No further concerns from my side.

@minxhe

minxhe commented Sep 23, 2026

Copy link
Copy Markdown
Author

Hi @Zakelly and @ferenc-csaky, could you help review this PR #29109 when you get a chance? It fixes --claimMode / --restoreMode being silently ignored in application mode and preserves explicitly configured modes through configuration round trips without a savepoint path.

Given your work on claim-mode handling and the CLI/application entrypoints, I’d appreciate your feedback on the approach. @Jackeyzhe has approved the latest revision, the existing review comments are addressed, and Azure CI is green. Thanks!

cc @weiqingy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-reviewed PR has been reviewed by the community.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants